-
-
Notifications
You must be signed in to change notification settings - Fork 225
Session Replay for iOS #4664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: version6
Are you sure you want to change the base?
Session Replay for iOS #4664
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## version6 #4664 +/- ##
===========================================
Coverage ? 73.18%
===========================================
Files ? 480
Lines ? 17421
Branches ? 3437
===========================================
Hits ? 12749
Misses ? 3821
Partials ? 851 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| { | ||
| // For replay to work on iOS, session tracking must be enabled in the Cocoa SDKt | ||
| options.AutoSessionTracking = false; | ||
| nativeOptions.EnableAutoSessionTracking = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how we now want to handle situations like this:
sentry-dotnet/src/Sentry/SentryClient.cs
Lines 350 to 362 in 9d1b4fe
| var hasTerminalException = processedEvent.HasTerminalException(); | |
| if (hasTerminalException) | |
| { | |
| // Event contains a terminal exception -> end session as crashed | |
| _options.LogDebug("Ending session as Crashed, due to unhandled exception."); | |
| scope.SessionUpdate = _sessionManager.EndSession(SessionEndStatus.Crashed); | |
| } | |
| else if (processedEvent.HasException()) | |
| { | |
| // Event contains a non-terminal exception -> report error | |
| // (this might return null if the session has already reported errors before) | |
| scope.SessionUpdate = _sessionManager.ReportError(); | |
| } |
... or this:
sentry-dotnet/src/Sentry/Internal/Hub.cs
Lines 376 to 384 in 9d1b4fe
| // Start a new session | |
| try | |
| { | |
| var sessionUpdate = _sessionManager.StartSession(); | |
| if (sessionUpdate is not null) | |
| { | |
| CaptureSession(sessionUpdate); | |
| } | |
| } |
If the CocoaSdk owns the session, do we still need to be able to start/stop sessions from the .NET SDK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these sessions are not the same as session replay. let's not conflate these APIs
Co-authored-by: GitHub <[email protected]> Co-authored-by: James Crosswell <[email protected]>
Co-authored-by: GitHub <[email protected]>
|
|
||
| ### Features | ||
|
|
||
| - Added experimental support for Session Replay on iOS ([#4664](https://github.com/getsentry/sentry-dotnet/pull/4664)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 🚫 The changelog entry seems to be part of an already released section
## 6.0.0.
Consider moving the entry to the## Unreleasedsection, please.
Part of #2136
This PR adds basic support for Session Replay, including associating replays with exceptions and traces.
It does not implement any way to configure granular masks... only basic options to mask all text / images or none are provided. That can be done in a separate PR.
Note
Note that there are no new tests here as the enabling functionality was added in #4133 (those tests cover this functionality already)